packages/webviewer/tests/main.test.ts (1)

21-51: ⚡ Quick win

Add callback-mode retry coverage.

Current suite validates only Promise-based fmFetch. Since callback-mode retry/error behavior was also changed, add tests for callback success-after-retry and callback failure after retries to lock in both contracts.

As per coding guidelines, test files should keep assertions inside it()/test() and use async/await patterns; extending coverage here should follow that same rule.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/webviewer/tests/main.test.ts` around lines 21 - 51, The test suite
currently only covers Promise-based fmFetch behavior but does not validate
callback-mode retry and error handling which was also modified. Add two new test
cases after the existing tests: one using it() that tests callback-mode success
after retry (similar structure to "retries when FileMaker is not yet available"
but invoke the callback-based fmFetch method and verify the callback receives
the response after retries), and another using it() that tests callback-mode
failure after three retries (verify the callback receives an error after
vi.advanceTimersByTimeAsync moves through the retry timeline). Both new tests
should follow the established pattern of using async/await, advancing timers
with vi.advanceTimersByTimeAsync(), and keeping all assertions inside the test
body.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

packages/webviewer/src/main.ts (1)

45-50: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Avoid converting callback-path retry exhaustion into an uncaught global exception.

Lines 47-49 rethrow via setTimeout, which makes failure uncatchable by callers and can crash the host runtime. Please surface this through a controllable channel (e.g., explicit error callback or Promise-returning API path) instead of async global throw.

As per coding guidelines, "Handle errors appropriately in async code with try-catch blocks".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/webviewer/src/main.ts` around lines 45 - 50, The error handling for
_execScriptWithFileMakerRetry is using setTimeout to rethrow errors
asynchronously, which creates an uncatchable global exception that can crash the
runtime. Replace this setTimeout pattern with a controllable error handling
approach by either passing the error to an explicit error callback parameter or
by returning the promise rejection in a way that allows callers to handle the
error through standard Promise error handling (e.g., by not catching it and
letting it propagate, or by explicitly handling it with a proper callback
mechanism). This ensures errors from the retry exhaustion are surfaced through a
channel that callers can control rather than becoming an uncaught global
exception.

Source: Coding guidelines

packages/webviewer/tests/main.test.ts (1)

43-51: ⚡ Quick win

Add callback-mode retry-exhaustion coverage.

Current tests cover Promise-mode exhaustion, but not the callback branch’s failure surface. Add one test for callback-mode exhaustion so future changes don’t regress that path.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/webviewer/tests/main.test.ts` around lines 43 - 51, Add a new test
case alongside the existing Promise-mode retry exhaustion test to cover
callback-mode retry exhaustion. Create a test that calls the fmFetch function
from "../src/main.ts" with a callback parameter instead of using Promise-based
handling, then advance timers by 1750ms to exhaust the three FileMaker
availability retries, and verify that the callback receives the expected error
indicating that 'window.FileMaker' was not available. This ensures the callback
branch of the retry logic is covered and protected against future regressions.

packages/webviewer/src/main.ts (2)

114-125: ⚡ Quick win

Refactor the retry loop to a bounded for...of flow.

Line 114 uses an indexed infinite loop. It works, but this violates the TS loop guideline and makes termination implicit.

♻️ Proposed refactor

 async function _execScriptWithFileMakerRetry(scriptName: string, data: unknown, cb: (arg0?: unknown) => void) {
-  for (let attempt = 0; ; attempt++) {
+  for (const delay of FM_FETCH_FILEMAKER_RETRY_DELAYS_MS) {
     try {
       _execScript(scriptName, data, cb);
       return;
     } catch (error) {
-      const delay = FM_FETCH_FILEMAKER_RETRY_DELAYS_MS[attempt];
-      if (!(delay && isFileMakerUnavailableError(error))) {
+      if (!isFileMakerUnavailableError(error)) {
         throw error;
       }
       await wait(delay);
     }
   }
+  _execScript(scriptName, data, cb);
 }

As per coding guidelines, "Prefer for...of loops over .forEach() and indexed for loops".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/webviewer/src/main.ts` around lines 114 - 125, The retry loop uses
an indexed infinite loop (for (let attempt = 0; ; attempt++)) which violates
TypeScript coding guidelines. Refactor this loop to use a for...of iteration
over the FM_FETCH_FILEMAKER_RETRY_DELAYS_MS array instead, making the loop
bounds explicit and termination conditions clear. The loop should iterate
through the delay values in the array, catching FileMaker unavailable errors and
retrying with the corresponding delay, then throwing any other errors. This will
replace the implicit termination of the current indexed infinite loop with an
explicit bounded iteration.

Source: Coding guidelines

---

5-5: ⚡ Quick win

Tie retryable-error classification to a single source of truth.

isFileMakerUnavailableError depends on a message fragment; if the thrown wording changes, retries silently stop. Consider deriving the thrown message and matcher from one shared constant (or an error code/type) to prevent drift.

Also applies to: 103-105

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/webviewer/src/main.ts` at line 5, The FILEMAKER_UNAVAILABLE_MESSAGE
constant and the isFileMakerUnavailableError function (which checks for a
message fragment) are decoupled, creating a maintenance risk where if one is
updated the other silently becomes inconsistent. Refactor to use a single source
of truth by ensuring the error message thrown when FileMaker is unavailable and
the matcher in isFileMakerUnavailableError both reference the same
FILEMAKER_UNAVAILABLE_MESSAGE constant, or alternatively consider using an error
code or type-based check instead of message string matching to decouple the
error classification from wording changes.